Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• 2d76189

r=me with commits squashed. Thanks!

@bors r=@Mark-Simulacrum

Commits squashed 🫡

@SadiinsoSnowfall: 🔑 Insufficient privileges: Not in reviewers

@bors r=@Mark-Simulacrum

📌 Commit 9c77de1 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

⌛ Testing commit 9c77de1 with merge 5c6b635...

💔 Test failed - checks-actions

Well, that's... surprising

Looks like LLVM doesn't make any optimizations in LoopVectorization in WASM for this, whereas it does in other architectures, preventing later improvements.

Is this by design ? If so, should I add something like @ ignore-wasm in the codegen test ?

//@ ignore-wasm FIXME: LLVM does not vectorize loops on WASM so this doesn't get optimized Should be okay

Hm, this makes me think that this issue isn't really fixed. We get it now as a combination of vectorization and unrolling, but that just pushes the constant up. If you replace 102 with say 1002, the issue still exists.

As far as I can tell, this is a phase ordering problem now. We successfully peel the first iteration now and bring the loop into an analyzable form, but this happens after induction variable simplification has already run, so we fail to simplify the loop further.

I'm curious about the status of this PR. Can anyone with some context share and what is needed to move it forward. I'll set it to waiting on author because I'm not completely sure it's ready for review

@rustbot author

The related issue hasn't been solved yet, so there's no need to add a codegen test yet I guess :/
Should I delete this PR ?

Yeah, I think we can close this PR for now ... the situation has improved, but the issue isn't fully fixed yet.